Avoid stale checked-in Cargo.lock files#1197
Avoid stale checked-in Cargo.lock files#1197ludfjig wants to merge 2 commits intohyperlight-dev:mainfrom
Conversation
| - name: Update simpleguest Cargo.lock | ||
| if: steps.check-ecosystem.outputs.is_cargo == 'true' | ||
| working-directory: src/tests/rust_guests/simpleguest | ||
| run: cargo update |
There was a problem hiding this comment.
Is cargo update more aggressive than we want here? IIUC cargo update would update all deps to the latest compatible version, whereas I wonder if cargo update simpleguest to do only the updates that are necessary to make the lockfiles consistent would be useful to make the PRs a bit more granular and isolate the effect of a dependency with a failing update?
There was a problem hiding this comment.
Perhaps building the guest would do the minimal necessary updates. I'm not sure cargo update simpleguest would work
There was a problem hiding this comment.
Actually cargo fetch should probably do it! (without the --locked)
|
@ludfjig Absolutely no pressure if you have higher-priority things, but is there anything we should do to progress this? It's only a tiny papercut, but I am constantly running into the out-of-date guest lock files because I now use Nix-vendored dependencies, which makes them a bit annoying (have to get a cargo executable not configured for vendoring, fix the lock file, wait for the dev environment to rebuild, etc). |
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
ludfjig
force-pushed
the
stale_cargo_lock
branch
from
a5aa0c9 to
9d06e6a
Compare
ludfjig
force-pushed
the
stale_cargo_lock
branch
from
9d06e6a to
916f7e3
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent stale, checked-in Cargo.lock files (especially for Rust test guests) by enforcing lockfile freshness in CI and adding automation to update guest lockfiles on Dependabot PRs.
Changes:
- Enforce
--lockedcargo fetchfor each Rust guest crate (simpleguest/dummyguest/witguest) in localjustCI-like checks. - Enforce the same guest lockfile checks in the reusable CI workflow (
dep_code_checks.yml). - Add a new workflow to auto-update and push guest
Cargo.lockfiles on Dependabot cargo PRs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Justfile |
Adds cargo fetch --manifest-path … --locked checks for each Rust guest crate. |
.github/workflows/dep_code_checks.yml |
Extends the existing lockfile freshness check to include each Rust guest crate. |
.github/workflows/dependabot-update-guest-locks.yml |
New workflow to regenerate guest lockfiles and push them back to Dependabot PR branches. |
ludfjig
force-pushed
the
stale_cargo_lock
branch
from
916f7e3 to
cc3b17e
Compare
ludfjig
force-pushed
the
stale_cargo_lock
branch
from
cc3b17e to
468d75d
Compare
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
ludfjig
force-pushed
the
stale_cargo_lock
branch
from
468d75d to
61a782a
Compare
ludfjig
requested review from
danbugs,
dblnz,
devigned,
jprendes,
marosset and
simongdavies
as code owners
2 participants
Avoid stale Cargo.lock being checked in. This is annoying because rust-analyzer will rebuild them locally and update Cargo.lock, resulting in locally modified Cargo.lock files, which can interfere with git operations (like git checkout).
Most complications come from the fact that dependabot will update dependencies (like hyperlight-common dependencies) without rebuilding guests resulting in the guest's Cargo.lock being outdated after a dependabot PR.f
I'm not 100% this will work after merging, so might require a future fix. Worst case is dependabot PRs won't get merged for a while, which is not the end of the world
Note: most of the scripting in this PR is made by copilot.
Prior art:
Note: since this will modify dependabot PRs, commands like
@dependabot rebasewon't work. Would need to do@dependabot recreateisntead, since dependabot won't rebase if somebody else touched the PR. This might make it a little more cumbersome to merge dependabot PRs...